Switch extension index entry format from "s4ext" to "json" | ⚠️ Changes removed from main #7629

jcfr · 2024-03-13T19:05:52Z

⚠️ Changes originally introduced through this pull-request have been removed from main by force pushing. Corresponding changes are superseded by #7709 ⚠️

Rational:

Commit message was incorrect
Changes were integrated using Squash & Merge instead of Rebase & Merge

In response to discussions during the Slicer developer hangout on March 12th, we have decided to revamp the organization of extension metadata in the extensions index, transitioning from the s4ext format to json.

Implements changes to the Extensions build-system, transitioning from .s4ext files to .json files for extension catalog entries. Additionally, it updates the ExtensionWizard and module templates to reflect this change and supports contributing .json files to the ExtensionsIndex.

As the category is now expected to be defined in the extension catalog entry file, the support for setting the "Category" directly from the ExtensionWizard UI has been removed.

While a .s4ext file is still generated in the built-tree and included in the extension package, this is considered an implementation detail to be addressed in subsequent commits.

The motivations behind these changes include:

Eliminate redundant and unused information from the "description" file.
Simplify programmatic parsing of the "description" files
Decouple the metadata organized in the extension CMakeLists.txt from
the ones organized in this repository and used to drive the build of extensions.
Enable Slicer maintainers to define and update the extension "category"
and "enabled" metadata independently of the upstream extension sources.

It removes the parsing and handling of metadata already defined in the extension CMakeLists.txt by the extension index build-system.

The metadata values now extracted from the <extensionname>.s4ext file locally generated in the extension build tree are the following:

"contributors"
"depends" (runtime dependencies)
"description"
"iconurl"
"homepage"
"screenshots"

The only metadata values propagated from the Slicer/ExtensionsIndex files now formatted as JSON files (<extensionname.json) are the following:

"scm", "scmurl" and "scmrevision"
"category"
"build_dependencies"
"build_subdirectory"
"enabled"

The extension name is still derived from the catalog entry filename.

Related pull requests:

https://github.com/matthiask/html-sanitizer#settings ↩

lassoan

Thanks a lot @jcfr the changes look good to me. I've just added a few small comments.

Extensions/CLIExtensionTemplate.json

Extensions/CMake/SlicerExtensionPackageAndUploadTarget.cmake

Extensions/CLIExtensionTemplate.json

Extensions/CMake/SlicerExtensionPackageAndUploadTarget.cmake

Docs/developer_guide/extensions.md

lassoan

Thank you, I've added some comments. I'm OK with most of them left unchanged, the only thing that I feel strongly about is that we should include the schema name in the file. I would rather not add lots of json files that are not clear what contain, especially because neither the filename, nor the file extension or file content is specific enough to confidently tell that it is an extensions catalog entry.

Docs/developer_guide/extensions.md

Extensions/CLIExtensionTemplate.json

Extensions/CMake/SlicerExtensionPackageAndUploadTarget.cmake

Extensions/CLIExtensionTemplate.json

Extensions/CMake/SlicerFunctionExtractExtensionDescription.cmake

Extensions/LoadableExtensionTemplate.json

Extensions/ScriptedLoadableExtensionTemplate.json

Extensions/ScriptedSegmentEditorEffectExtensionTemplate.json

Extensions/SuperBuildExtensionTemplate.json

lassoan

Thank you, it looks good, just marked a few trivial things. If there is any trouble with removing "NA" then it is fine to leave it there.

Docs/developer_guide/extensions.md

Extensions/CLIExtensionTemplate.json

Extensions/LoadableExtensionTemplate.json

Extensions/ScriptedLoadableExtensionTemplate.json

Extensions/ScriptedSegmentEditorEffectExtensionTemplate.json

Extensions/SuperBuildExtensionTemplate.json

jcfr · 2024-03-21T01:27:21Z

If I understand correctly, we should remove the category from here then.

Ditto.

This should be addressed in the commit ENH: Ensure extension category from catalog entry file is used, I still need to test the approach.

Also need to display warnings (when using Slicer >= 5.8) to suggest removal of EXTENSION_CATEGORY and EXTENSION_ENABLED in extension CMakeLists.txt if the corresponding variables are set.

lassoan · 2024-03-28T22:30:48Z

@jcfr let me know if this is ready for final review

jcfr · 2024-03-29T00:09:26Z

This is ready for review.

That said, I still need to update the ExtensionIndex based in the new schema and create PR on existing Extension.

jcfr · 2024-04-23T15:25:34Z

Let me know if this is ready for final review

@lassoan This is now ready for final review.

Anticipating the simplification of the metadata used to describe an extension Simplifies handling of metadata catalog entry, this removes the parsing and handling of metadata already defined in the extension CMakeLists.txt by the extension index build-system. The following metadata values are now extracted from the `<extensionname>.s4ext` file locally generated in the extension build tree: - "category" - "contributors" - "enabled" - "depends" (runtime dependencies) - "description" - "iconurl" - "homepage" - "screenshots" The only metadata values propagated from the Slicer/ExtensionsIndex files are the following: - "scm", "scmurl" and "scmrevision" - "depends" (built-time dependencies) - "build_subdirectory" The extension name is still derived from the catalog entry filename.

This commit implements changes to the Extensions build-system, transitioning from `.s4ext` files to `.json` files for extension index entries. Additionally, it updates the ExtensionWizard and module templates to reflect this change and supports contributing `.json` files to the ExtensionsIndex. As the category is now expected to be defined in the extension catalog entry file, the support for setting the "Category" directly from the ExtensionWizard UI has been removed. While a .s4ext file is still generated in the built-tree and included in the extension package, this is considered an implementation detail to be addressed in subsequent commits. The motivations behind these changes include: * Eliminate redundant and unused information from the "description" file. * Simplify programmatic parsing of the "description" files * Decouple the metadata organized in the extension `CMakeLists.txt` from the ones organized in this repository and used to drive the build of extensions. * Enable Slicer maintainers to define and update the extension "category" and "enabled" metadata independently of the upstream extension sources.

…ies"

Co-authored-by: Andras Lasso <lasso@queensu.ca>

pieper

Impressive work Jc 👍

I don't see anything obviously wrong so if you are comfortable I think we should go ahead. We'll know for sure once we try it for real.

jcfr · 2024-04-23T21:07:56Z

Thanks for the cursory check.

if you are comfortable I think we should go ahead

On the metroplex factory, I was able to locally build the index and publish an extension so we should be in good shape.

Co-authored-by: Andras Lasso <lasso@queensu.ca>

…r list

jcfr · 2024-04-23T21:22:41Z

While ensuring Slicer/ExtensionsIndex#2011 was also passing, I realized I missed one update¹ to the schema, this is now fixed.

@pieper I would appreciate a re-approval

https://github.com/Slicer/Slicer/compare/e6151f39214aa197d77b7a6b083d9c4c181ceb8e..783f4015450605930ae9d7f68cf3f1ea7d6418de ↩

pieper

👍

jcfr force-pushed the transition-extensions-index-build-system-from-s4ext-to-json branch 2 times, most recently from b9f087c to 9722894 Compare March 13, 2024 22:06

lassoan reviewed Mar 13, 2024

View reviewed changes

jcfr force-pushed the transition-extensions-index-build-system-from-s4ext-to-json branch 3 times, most recently from 751498c to eeba6b0 Compare March 14, 2024 03:15

jcfr commented Mar 14, 2024

View reviewed changes

Extensions/CMake/SlicerExtensionPackageAndUploadTarget.cmake Show resolved Hide resolved

jcfr mentioned this pull request Mar 19, 2024

Simplify extension manager build-system #7644

Merged

This comment was marked as outdated.

Sign in to view

jcfr force-pushed the transition-extensions-index-build-system-from-s4ext-to-json branch from eeba6b0 to 89a306e Compare March 19, 2024 16:57

This comment was marked as outdated.

Sign in to view

jcfr mentioned this pull request Mar 19, 2024

CI: Transition from s4ext to json Slicer/ExtensionsIndex#2011

Merged

2 tasks

jcfr force-pushed the transition-extensions-index-build-system-from-s4ext-to-json branch from 89a306e to 7b07924 Compare March 19, 2024 21:57

jcfr commented Mar 19, 2024

View reviewed changes

Docs/developer_guide/extensions.md Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

lassoan reviewed Mar 20, 2024

View reviewed changes

jcfr force-pushed the transition-extensions-index-build-system-from-s4ext-to-json branch 2 times, most recently from 0404746 to 3a58c0b Compare March 20, 2024 20:05

lassoan previously approved these changes Mar 20, 2024

View reviewed changes

jcfr dismissed lassoan’s stale review via 5c79b1f March 21, 2024 01:22

jcfr force-pushed the transition-extensions-index-build-system-from-s4ext-to-json branch from 3a58c0b to 5c79b1f Compare March 21, 2024 01:22

jcfr force-pushed the transition-extensions-index-build-system-from-s4ext-to-json branch 2 times, most recently from 14c8607 to f96c71c Compare March 28, 2024 21:02

jcfr force-pushed the transition-extensions-index-build-system-from-s4ext-to-json branch from f96c71c to 0a243fe Compare April 11, 2024 16:02

jcfr force-pushed the transition-extensions-index-build-system-from-s4ext-to-json branch from 0a243fe to 3f37a26 Compare April 20, 2024 05:31

jcfr mentioned this pull request Apr 22, 2024

Update extension to ensure successful build & distribution SlicerIGT/Scoliosis#4

Merged

jcfr requested review from lassoan and pieper April 23, 2024 15:24

jcfr requested a review from jamesobutler April 23, 2024 19:19

jcfr force-pushed the transition-extensions-index-build-system-from-s4ext-to-json branch from 3f37a26 to 5a7fc3d Compare April 23, 2024 19:52

jcfr and others added 3 commits April 23, 2024 16:47

ENH: Rename catalog entry metadata from "depends" to "build_dependenc…

4a0a17a

…ies"

ENH: Rename catalog entry "scm" metadata to be consistently cased

ba67930

Co-authored-by: Andras Lasso <lasso@queensu.ca>

jcfr force-pushed the transition-extensions-index-build-system-from-s4ext-to-json branch from 5a7fc3d to e6151f3 Compare April 23, 2024 20:48

pieper previously approved these changes Apr 23, 2024

View reviewed changes

jcfr enabled auto-merge (rebase) April 23, 2024 21:08

jcfr disabled auto-merge April 23, 2024 21:14

jcfr and others added 3 commits April 23, 2024 17:20

ENH: Add extension catalog entry file schema

cff2b1d

Co-authored-by: Andras Lasso <lasso@queensu.ca>

ENH: Support specifying extension description metadata with newlines

690b46d

ENH: Support specifying screenshot url extension metadata as string o…

783f401

…r list

jcfr dismissed pieper’s stale review via 783f401 April 23, 2024 21:20

jcfr force-pushed the transition-extensions-index-build-system-from-s4ext-to-json branch from e6151f3 to 783f401 Compare April 23, 2024 21:20

jcfr requested a review from pieper April 23, 2024 21:22

pieper approved these changes Apr 23, 2024

View reviewed changes

pieper enabled auto-merge (squash) April 23, 2024 22:25

pieper merged commit 59f9126 into Slicer:main Apr 23, 2024
5 checks passed

jcfr mentioned this pull request Apr 24, 2024

Switch extension index entry format from "s4ext" to "json" #7709

Merged

jcfr changed the title ~~Switch extension index entry format from "s4ext" to "json"~~ Switch extension index entry format from "s4ext" to "json" | ⚠️ Changes removed from main Apr 24, 2024

jcfr deleted the transition-extensions-index-build-system-from-s4ext-to-json branch April 24, 2024 02:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch extension index entry format from "s4ext" to "json" | ⚠️ Changes removed from main #7629

Switch extension index entry format from "s4ext" to "json" | ⚠️ Changes removed from main #7629

jcfr commented Mar 13, 2024 •

edited

lassoan left a comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

lassoan left a comment

lassoan left a comment

jcfr commented Mar 21, 2024 •

edited

lassoan commented Mar 28, 2024

jcfr commented Mar 29, 2024

jcfr commented Apr 23, 2024

pieper left a comment

jcfr commented Apr 23, 2024

jcfr commented Apr 23, 2024

pieper left a comment

Switch extension index entry format from "s4ext" to "json" | ⚠️ Changes removed from main #7629

Switch extension index entry format from "s4ext" to "json" | ⚠️ Changes removed from main #7629

Conversation

jcfr commented Mar 13, 2024 • edited

Footnotes

lassoan left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

lassoan left a comment

Choose a reason for hiding this comment

lassoan left a comment

Choose a reason for hiding this comment

jcfr commented Mar 21, 2024 • edited

lassoan commented Mar 28, 2024

jcfr commented Mar 29, 2024

jcfr commented Apr 23, 2024

pieper left a comment

Choose a reason for hiding this comment

jcfr commented Apr 23, 2024

jcfr commented Apr 23, 2024

Footnotes

pieper left a comment

Choose a reason for hiding this comment

jcfr commented Mar 13, 2024 •

edited

jcfr commented Mar 21, 2024 •

edited